Skip to content

feat(fabric): add tenant-wildcard comms subscribe (RIG-3107) - #903

Open
rigel-mintaka wants to merge 3 commits into
compass-managed/rig-3107-nats-fabricfrom
compass-managed/rig-3107-fabric-wildcard-subscribe
Open

feat(fabric): add tenant-wildcard comms subscribe (RIG-3107)#903
rigel-mintaka wants to merge 3 commits into
compass-managed/rig-3107-nats-fabricfrom
compass-managed/rig-3107-fabric-wildcard-subscribe

Conversation

@rigel-mintaka

@rigel-mintaka rigel-mintaka commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

This PR is part of a stack containing 6 PRs:

  1. main
  2. chore(go): raise module floor to 1.26 + absorb modernize sweep (RIG-3107) #876
  3. feat(fabric): add NATS EventFabric + RunnerFabric seams (RIG-3107) #877
  4. "feat(fabric): add tenant-wildcard comms subscribe (RIG-3107)" (this PR)
  5. feat(fabric): add the RoutingFabric seam for binding-cache invalidation (RIG-3108) #955
  6. refactor(store): maintain updated_at with a trigger, not by hand (RIG-3495) #989
  7. feat(store): add the durable session_bindings table and its store methods (RIG-3108) #990

The T3 delivery consumer is a per-Server singleton serving every tenant, so it
must receive one event kind across all tenants. Each event publishes to a
concrete compass.<tenant>.comms.<kind>, so the singleton needs a
tenant-wildcard subscribe.

  • CommsWildcardSubject(kind) builds compass.*.comms.<kind>: the tenant token
    is the literal *, the kind stays concrete and ValidSubjectToken-checked so
    a wildcard kind can never widen the subject to all kinds.
  • (*Fabric).SubscribeKind(ctx, kind, fn) subscribes on that subject via one
    durable queue-group consumer, sharing the exact ack/park/teardown machinery of
    Subscribe (both now route through a private subscribeSubject helper).
    Subscribe keeps its strict concrete-only grammar; the wildcard is reachable
    only through SubscribeKind's own validated builder.
  • Publish is untouched and still cannot target a wildcard: it derives its
    subject from the ref, and EventRef.valid rejects a * tenant.
  • DLQ provenance + per-message logs record msg.Subject() (the concrete
    delivered subject), so a parked message on the wildcard consumer keeps its
    tenant.
  • design.md's frozen EventFabric interface + SUBJECTS.md document the new
    read-side seam; the wildcard and concrete consumers are independent durables,
    so a later migration must retire the concrete subscribes rather than run both.

Co-authored-by: Matt Wilkinson matt@rigel.build

@linear-code

linear-code Bot commented Sep 5, 2026

Copy link
Copy Markdown

RIG-3107

@rigel-mintaka
rigel-mintaka marked this pull request as ready for review September 5, 2026 19:53
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown

Compass engineering docs preview: https://compass-managed-rig-3107-fab.compass-eng-docs.pages.dev

Deployed from compass-managed/rig-3107-fabric-wildcard-subscribe at fa9f5f4.

Changed pages:

@trunk-io

trunk-io Bot commented Sep 6, 2026

Copy link
Copy Markdown

❌ This stack was removed from the merge queue because it failed tests. PR #917 was used for testing. See more details here.

Failed Required Status Conclusion
rollup Failure
  • To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

@rigel-mintaka
rigel-mintaka force-pushed the compass-managed/rig-3107-fabric-wildcard-subscribe branch from c6e90d7 to 5694f31 Compare September 6, 2026 22:26
@rigel-mintaka
rigel-mintaka force-pushed the compass-managed/rig-3107-fabric-wildcard-subscribe branch from 5694f31 to 8671bf8 Compare September 6, 2026 23:18
@rigel-mintaka
rigel-mintaka force-pushed the compass-managed/rig-3107-fabric-wildcard-subscribe branch from 67a6f15 to 17874a2 Compare September 7, 2026 00:56
@rigel-mintaka
rigel-mintaka force-pushed the compass-managed/rig-3107-fabric-wildcard-subscribe branch from 17874a2 to edf7798 Compare September 7, 2026 14:42
@rigel-mintaka
rigel-mintaka force-pushed the compass-managed/rig-3107-fabric-wildcard-subscribe branch from edf7798 to 0c29465 Compare September 7, 2026 19:39
rigel-mintaka and others added 3 commits September 7, 2026 18:17
The T3 delivery consumer is a per-Server singleton serving every tenant, so it
must receive one event kind across all tenants. Each event publishes to a
concrete `compass.<tenant>.comms.<kind>`, so the singleton needs a
tenant-wildcard subscribe.

- `CommsWildcardSubject(kind)` builds `compass.*.comms.<kind>`: the tenant token
  is the literal `*`, the kind stays concrete and `ValidSubjectToken`-checked so
  a wildcard kind can never widen the subject to all kinds.
- `(*Fabric).SubscribeKind(ctx, kind, fn)` subscribes on that subject via one
  durable queue-group consumer, sharing the exact ack/park/teardown machinery of
  `Subscribe` (both now route through a private `subscribeSubject` helper).
  `Subscribe` keeps its strict concrete-only grammar; the wildcard is reachable
  only through `SubscribeKind`'s own validated builder.
- Publish is untouched and still cannot target a wildcard: it derives its
  subject from the ref, and `EventRef.valid` rejects a `*` tenant.
- DLQ provenance + per-message logs record `msg.Subject()` (the concrete
  delivered subject), so a parked message on the wildcard consumer keeps its
  tenant.
- design.md's frozen `EventFabric` interface + SUBJECTS.md document the new
  read-side seam; the wildcard and concrete consumers are independent durables,
  so a later migration must retire the concrete subscribes rather than run both.

Co-authored-by: Matt Wilkinson <matt@rigel.build>
… (RIG-3107)

The subscribe path decoded an EventRef and handed it to the subscriber without
comparing the ref's tenant to the subject the message arrived on, so the
tenant-isolation invariant was enforced on the write side only. Publish derives
the required subject from the ref and refuses a mismatch, but Publish is not the
only writer the stream can have: COMPASS_COMMS is shared and the server carries
no per-tenant authorization yet, so any client that reaches the client port can
put arbitrary bytes on any comms subject.

That mattered because EventRef's contract directs a subscriber to re-read its
row under ref.Tenant WITHOUT consulting the subject. A ref naming tenant-a
delivered on tenant-b's subject therefore drove a tenant-b-scoped subscriber to
read a tenant-a row. SubscribeKind makes it the primary path rather than a
latent gap: its consumer spans every tenant, which leaves the payload's tenant
field as the only scope a delivery carries.

handleEvent now rebuilds the wanted subject from the ref and parks on a
mismatch. A mismatch is as unprocessable as an undecodable payload, so it parks
rather than retrying, and the DLQ entry keeps the concrete delivered subject so
an operator can see which tenant was targeted.

Also wires nats.ErrorHandler, without which the Runner plane's documented
dropped-and-reported semantic was only dropped: nats.go reports a full channel
subscription through the async error callback and the defaults install none, so
the fabric's one lossy path dropped events with no log line, no metric and no
error.
…mer report (RIG-3107)

Two guarantees from the review loop shipped without tests, and both fail
invisibly.

The subject cross-check parks a ref whose tenant disagrees with its delivered
subject, deciding that by rebuilding the subject with the same CommsSubject call
Publish uses. That the two agree was emergent rather than tested: a one-sided
change — normalizing, lowercasing or trimming the tenant token on one side only
— would park every legitimate delivery on every tenant, a silent fleet-wide
denial of delivery whose only signal is a filling DLQ. The forged-ref test
covers the negative case and the wildcard tests only exercise SubscribeKind, so
the highest-traffic path had no defense. TestLegitimateRefsSurviveTheSubjectCrossCheck
publishes across the token space ValidSubjectToken admits — dashes,
underscores, mixed case, a uuid — asserts each is delivered through a concrete
Subscribe, and asserts nothing legitimate reaches the DLQ.

The async error handler is the Runner plane's only report of its only lossy
path, and nats.go installs none by default while its own fallback writes to raw
stderr rather than the fabric's logger. Dropping or shadowing the option would
silently return the plane to losing events invisibly with no test failing.
TestAsyncErrorHandlerReportsSlowConsumers invokes the installed handler directly
— nats.go dispatches it on its own goroutine holding no lock, so calling it is
faithful and carries no timing dependence — covering both the nil-subscription
branch that the whole drain path takes and the branch that must name the subject.
TestCallerErrorHandlerWins pins the Config.Options precedence, which is correct
to allow here because the handler is log-only, unlike the ClosedHandler that
Close deliberately avoids depending on.
@rigel-mintaka
rigel-mintaka force-pushed the compass-managed/rig-3107-fabric-wildcard-subscribe branch from 0c29465 to fa9f5f4 Compare September 7, 2026 22:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants